fix(service-automation): decide the claim capability before the compare-and-set, never after it - #16128

Refs #15832 (Note 2)

⛔ Not Fixes. Note 1 of that card landed separately in #16031 (1157e7b72, the hot-cache eviction in engine.ts) and is not touched here — git show --stat 1157e7b72 is engine.ts + one pin + one changeset, and this branch's diff contains no engine.ts path. The PM seat releases the card.

The defect this closes, and which of its two costs is the reason

ObjectStoreSuspendedRunStore.claimSuspension decided 'unsupported' from the shape of the return value — one line after the compare-and-set had already gone out:

const affected = await this.engine.delete(TABLE, { where, multi: true, context: SYSTEM_CTX });
if (typeof affected !== 'number') { this.warnClaimUnsupported(…); return 'unsupported'; }

'unsupported' means "no cross-replica advance guarantee is offered by this store". Said there, it is a statement about a write that has already landed against the shared row: the conditional delete was performed and its verdict discarded, AutomationEngine.claimAdvance reads 'unsupported' as unguarded, and a replica that actually lost the claim (0 rows affected) resumed anyway — the doubled side effect #14333 exists to prevent, on the one composition that declares itself unable to prevent it.

⛔ The redundant round-trip is the other cost and is not why this changed. A change that only removed it would leave four of the pins below red.

Re-confirmed on this head, not carried on the card's word

The card was verified against origin/main d4f9b2a9d; this branch is merged up to 9b459b791. Read on the current tree:

• claimAdvance (engine.ts:2269): if (outcome === 'unsupported') { warnAdvanceClaimDegraded(…); return { kind: 'unguarded' }; }
• the resume path (engine.ts:5490): await this.forgetSuspendedRun(run, 'resumed', claim.kind === 'claimed') — so an unguarded verdict passes false, and forgetSuspendedRun's if (this.store && !durableRecordAlreadyConsumed) issues the second, unconditional store.delete(runId). Both still hold.

⚠️ What a pre-write probe can determine — and what it cannot

Stated plainly, because a probe that only looks like it decides in advance would be worse than the defect.

ObjectQL.delete declares Promise of any (packages/objectql/src/engine.ts:12996) and what surfaces is opCtx.result, which any middleware may rewrite per call. So "does this engine's multi-delete return a count" has no contractual answer to look up and no read-only instrument to measure. The only thing that answers is a call down the same route — a where carrying keys besides id together with multi: true, which is what dispatches to driver.deleteMany. A probe can therefore observe the route once; it can never promise what the next call resolves to.

⇒ The probe alone does not close this card. It is one of two arms:

1. Before the write — a one-time capability probe. Once per store instance, down the same route, against a sentinel predicate that matches no row: the same value in id, node_id and correlation at once, so a row would have to carry that one string in all three columns to match. An engine that resolves something other than a count is refused with nothing consumed, which is what makes claimAdvance's unguarded reading true when it is taken. Concurrent first claims share one probe; a probe that throws is deliberately not memoized (a store unreachable for one second must not answer for the life of the process), and the rejection reaches claimAdvance, which already maps it to STORE_UNAVAILABLE — the same answer the claim itself produced when it was the call that threw.

2. After the write — 'unsupported' is retired as an answer. If a probed-counting engine still resolves a non-count for a real claim, the compare-and-set is committed and its verdict is unrecoverable: a winner and a loser both find the row gone, so no follow-up read can tell them apart. That is UNKNOWN, not unguarded. The store throws; claimAdvance catches it and answers STORE_UNAVAILABLE, whose text is already written for exactly this fact ("a failure can arrive after a committed delete"), and the resume is refused. ⇒ There is now no path on which this store answers 'unsupported' after a delete carrying the run's condition has been issued — whatever the probe concluded.

⛔ Refusing is not free and is not pretended to be: a claim that in fact won is then stranded until an operator retries. That is the deliberate direction (#14333's premise is that a doubled side effect is the worse outcome), and it is reachable only on an engine that answers inconsistently between the probe and the claim.

Alternatives considered, and why not

• A throwaway ROW to probe against (insert, then multi-delete it). It answers a strictly stronger question — that the count counts, not merely that it is a number — while typeof affected !== 'number' is the whole of the condition this store branches on. It buys an INSERT on a platform object and a stranded row whenever a process dies between the two statements. Not taken.
• An empty $in or another "matches nothing by construction" operator. Reads as the safer spelling and is the more dangerous one: whether an empty IN compiles is a driver-by-driver question, and the failure direction of a builder that drops an empty clause is a DELETE over the whole table. Three ANDed equalities compile the same way everywhere and fail closed.
• A declared capability on the store, or refusing at construction. Either default is wrong on its own: assuming counted leaves the harm, assuming uncounted degrades every deployment. The residual arm above is what makes a default unnecessary.

⚠️ What this does NOT do

It does not give an uncounted engine the guarantee, and no pin here claims it does — one case asserts the opposite deliberately, so a later reader does not over-read the others. The count is contracted one layer down (IDataDriver.deleteMany, packages/spec/src/contracts/data-driver.ts:269, Promise of number) and erased to any at the engine boundary this store talks to. That gap is #16033.

⚠️ A scope conflict for the PM, not a finding of mine. Comment 5554626144 split Note 2 out as #16033 on a (b) cross-lane judgement; the newer release comment 5555903647 reverses that and keeps Note 2 on #15832 in domain:services. I built to the newer one and touched nothing under packages/objectql or packages/spec. #16033 was OPEN, unassigned and had zero comments when I claimed, so there was no in-flight duplicate work. Whether it is now superseded or kept for the engine-level declaration is the PM's call.

Pins — 13 cases, and each population named

New file packages/services/service-automation/src/suspended-run-claim-probe.test.ts. Three populations:

1. Store level, one fake data engine whose delete and update are bound to the producer's own dispatch predicates (assertEngineDeleteDispatch / assertEngineUpdateDispatch), so the double cannot accept a call the real engine refuses. Three engine return shapes are driven: a counting one; a uniformly non-counting one (undefined); and an inconsistent one that counts for the probe and does not for the claim — the only shape on which arm 2 is reachable.
2. Two AutomationEngine replicas over ONE ObjectStoreSuspendedRunStore, over one of those fakes. Two replicas is the whole modelled fleet — the smallest number on which "the loser resumes too" is observable — and the side effects are counted off a shared ledger, not off call spies.
3. A real kernel: ObjectKernel + ObjectQLPlugin + SqlDriver (better-sqlite3) over a real sys_automation_run table, for the two facts a fake cannot witness — that the shipped composition takes the counted path, and that the probe's predicate consumes nothing there.

⭐ The pin the card asks for is the losing replica's verdict, not that a function was called: "THE HARM: a replica whose claim verdict is unreadable is REFUSED, not resumed" asserts fired === [] and both replicas answering STORE_UNAVAILABLE.

⛔ NOT MEASURED, named rather than implied: driver-mongodb (the mongod binary download is refused by this environment's egress proxy), driver-sql on postgres and mysql, and Turso against a hosted endpoint — only better-sqlite3 is installable in this container. Third-party engines and drivers by definition.

Verification — every exit code captured after a single redirected command, never through a pipe

All readings on the final commit 90f388825 unless stated.

Two gates first answered 3 = PREREQUISITE NOT MET (built output absent) and were re-run after pnpm --filter @objectstack/service-automation build / pnpm build rather than reported as failures: check:dual-build-cjs-loads → 0, check:type-check-debt → 0 (12 ledger entr(ies) re-measured … none above its recorded number).

The family was re-derived on the final head, and that mattered: the first derivation returned 54 families, and adding scripts/engine-double-contract.pinned.json to the change set moved it to 62. The earlier derivation also reported STALE TREE; origin/main was merged in (51a362c93) and the derivation re-run until it named no staleness.

check:engine-double-contract was red at first with two problems, both fixed the non-weakening way: the new fake's update() now routes through assertEngineUpdateDispatch, and node scripts/check-engine-double-contract.mjs --write recorded the new pinned rows (2 added or grown, 0 lost). ⛔ The shrink-only baseline was not touched.

Ablation — implementation committed first, three legs

Each leg: mutate on disk → prove the mutation by an anchored occurrence count and a git hash-object delta (a no-op hash aborts the leg) → run → restore via git checkout HEAD -- ABSOLUTE_PATH under trap … EXIT INT TERM → prove the restore by blob equality against the HEAD blob and an empty git diff HEAD. No rebuild is owed between mutation and reading: the pin imports ./suspended-run-store.js, a same-package relative specifier, so no dist sits between the edit and the run — which the red readings themselves demonstrate.

All three restores reported blob match=YES diff-HEAD-empty=YES.

⭐ Leg B is the card, reproduced by run on this branch: with the second arm removed, the doubled side effect returns.

Housekeeping

• ⛔ packages/services/service-automation/src/engine.ts is untouched — the fence in the brief holds, and no part of this fix needed it.
• 90f388825 is a preservation commit the PM seat pushed after a container restart killed this seat mid-run; its content is this branch's work. It carries no closing keyword, and history was not rewritten.
• Changeset: patch on @objectstack/service-automation.

⛔ No claim is made here about this PR's CI state.

Generated by Claude Code